Skip to content

Fix GrumPHP config paths for global dev-tools sync#295

Merged
coisa merged 2 commits intomainfrom
bugfix/288-global-grumphp-sync
Apr 30, 2026
Merged

Fix GrumPHP config paths for global dev-tools sync#295
coisa merged 2 commits intomainfrom
bugfix/288-global-grumphp-sync

Conversation

@coisa
Copy link
Copy Markdown
Contributor

@coisa coisa commented Apr 30, 2026

Related Issue

Closes #288

Motivation / Context

  • global dev-tools sync was persisting DevTools-owned GrumPHP config paths in consumer composer.json, which breaks when the path is machine-specific
  • Git hooks were the only consumer of that managed path, so the path should stay local to the installed hooks rather than committed metadata

Changes

  • remove deprecated DevTools-managed extra.grumphp.config-default-path values while preserving consumer-owned GrumPHP settings
  • render packaged Git hooks with the active DevTools grumphp.yml path at install time, while still preferring a project-local grumphp.yml
  • add dedicated synchronizer and hook renderer coverage, plus docs/changelog updates for the legacy transition

Verification

  • composer dev-tools
  • Focused command(s): composer dev-tools tests -- --filter=(ManagedConfigPathSynchronizerTest|UpdateComposerJsonCommandTest|GitHooksCommandTest|HookContentRendererTest|PackagedHooksTest|SyncCommandTest)
  • Manual verification:

Documentation / Generated Output

  • README updated
  • docs/ updated
  • Generated or synchronized output reviewed

Changelog

  • Added a notable CHANGELOG.md entry

Reviewer Notes

  • .github/wiki remained dirty locally before this work and was not included in the commit.

@github-actions
Copy link
Copy Markdown
Contributor

@coisa coisa merged commit 725f6f7 into main Apr 30, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Released in PHP Fast Forward Project Apr 30, 2026
@coisa coisa deleted the bugfix/288-global-grumphp-sync branch April 30, 2026 02:41
github-actions Bot added a commit that referenced this pull request Apr 30, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ef340b0d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

$composerJsonData['extra'] = $extra;
$composerJsonData['extra'] = $this->managedConfigPathSynchronizer->synchronize(
$extra,
\dirname($file),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Normalize composer.json base directory before sync

updatedComposerJsonContents() passes dirname($file) directly into ManagedConfigPathSynchronizer::synchronize(). With the default --file value (composer.json), this becomes . (a relative path), and isManagedConfigPath() then calls Path::makeRelative() with an absolute managed path and a relative base path, which raises InvalidArgumentException. In practice this causes default update-composer-json/dev-tools:sync runs to fail instead of updating metadata.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Released

Development

Successfully merging this pull request may close these issues.

Fix GrumPHP config paths for global dev-tools sync

1 participant